Skip to content

chore: distribute cjs @W-21382713#5719

Merged
wjhsf merged 1 commit intomasterfrom
wjh/oops-distribute-cjs
Feb 27, 2026
Merged

chore: distribute cjs @W-21382713#5719
wjhsf merged 1 commit intomasterfrom
wjh/oops-distribute-cjs

Conversation

@wjhsf
Copy link
Contributor

@wjhsf wjhsf commented Feb 27, 2026

I intended to do this with the v9.0.0 release, but apparently forgot.

Details

Does this pull request introduce a breaking change?

  • 😮‍💨 No, it does not introduce a breaking change.
  • 💔 Yes, it does introduce a breaking change.

Does this pull request introduce an observable change?

  • 🤞 No, it does not introduce an observable change.
  • 🔬 Yes, it does include an observable change.

GUS work item

@wjhsf
chore: distribute cjs
4aefba3 
intended to do this with the v9.0.0 release, but apparently forgot
@wjhsf wjhsf requested a review from a team as a code owner February 27, 2026 16:49
@wjhsf wjhsf changed the title chore: distribute cjs chore: distribute cjs @W-21382713 Feb 27, 2026
rax-it
rax-it reviewed Feb 27, 2026
View reviewed changes
Copy link
Contributor

@rax-it rax-it left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wjhsf
Do we need something like:

"exports": {
  ".": {
    "import": "./dist/index.js",
    "require": "./dist/index.cjs"
  }
}

For lwc-transformer to import it correctly? I'm not sure how the .cjs file is being resolved?

@wjhsf
Copy link
Contributor Author

wjhsf commented Feb 27, 2026

I tried adding exports as part of the original v9 release, but it broke too many other things. For v9, dist/index.cjs exists. For v8, dist/index.cjs doesn't exist, so node goes through its list of assumed file extensions. dist/index.cjs.js does exist, so the import works. (There's no special meaning to the .cjs in the file name. Node treats it the same way it would treat dist/index or dist/foo.)

rax-it
rax-it reviewed Feb 27, 2026
View reviewed changes
Copy link
Contributor

@rax-it rax-it left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like in v8 we had:

"main": "dist/index.cjs.js",
"module": "dist/index.js",

which is probably why node was able to resolve it, and we don't have this in v9.
Sorry, I am still not convinced that this will work.
Can you link this locally to lwc-test and make sure it works?

@wjhsf
Copy link
Contributor Author

wjhsf commented Feb 27, 2026

The "module" field is a convention for bundlers; node itself doesn't use it in the lookup algorithm. require("@lwc/engine-dom/dist/index.cjs") works for both v8 and v9.

@rax-it
Copy link
Contributor

rax-it commented Feb 27, 2026

The "module" field is a convention for bundlers; node itself doesn't use it in the lookup algorithm. require("@lwc/engine-dom/dist/index.cjs") works for both v8 and v9.

Sure that will work but will this import without nested import work?
Do we need to update the jest-transformer?

I don't think explicitly requiring .cjs extension is the solution but as you mentioned it was causing other issues so I am willing to give this a shot.

@wjhsf
Copy link
Contributor Author

wjhsf commented Feb 27, 2026

Sure that will work but will this import without nested import work?

No, this change will not make require("@lwc/compiler") work, but that is not the goal of this PR. The goal of this PR is simply to publish .cjs files. To fix the issues with lwc-test, there are other changes in salesforce/lwc-test#404.

@wjhsf wjhsf merged commit bb244a6 into master Feb 27, 2026
47 of 48 checks passed
@wjhsf wjhsf deleted the wjh/oops-distribute-cjs branch February 27, 2026 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rax-it rax-it rax-it approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wjhsf @rax-it